Comment on attachment 8950853 [details] Bug 1438106 - Fix a resource leak on fd https://reviewboard.mozilla.org/r/220088/#review226036 ::: toolkit/crashreporter/nsExceptionHandler.cpp:3134 (Diff revision 1) > if (fd) { > AnnotationTable exceptionTimeAnnotations; > ReadAndValidateExceptionTimeAnnotations(fd, exceptionTimeAnnotations); > PR_Close(prFd); > if (!AppendExtraData(extra, exceptionTimeAnnotations)) { > + fclose(fd); You should be able to put this next to the `PR_Close(prFd)`. Did I misunderstand the Windows semantics of `fdopen(_open_osfhandle(handle))`? I thought you only needed to close the original handle, but I guess based on this PR you need to close both the handle and the fd.

> You should be able to put this next to the `PR_Close(prFd)`. I think Coverity thinks that you should always call fclose() even if fopen failed. Here, if I move it, we will have twice fclose(fd) (which might not be an issue). Just let me know what you prefer!

Hmmm, I'm not sure I follow; are we sure that we need to close both the HANDLE and the fd? You've highlighted that in cases where fdopen fails we don't close the HANDLE though, same with cases where _open_osfhandle fails. I'm not sure if either of these are possible.

Closing: <Alex_Gaynor> I'm looking at our other callers of that function to see what we do <Alex_Gaynor> yeah, it looks like you only need to close one of them <Sylvestre> so, false positive <Alex_Gaynor> I think so